objectcache: add $flags argument to BagOStuff::delete()

This makes it consistent with set() and merge(). Also, one subclass
was already using the field in this manner.

Clean up the code related to WRITE_SYNC in SqlBagOStuff.

Change-Id: I0fb84f4475311889507d3ef98afd4476fb81174f

diff --git a/includes/libs/objectcache/APCBagOStuff.php b/includes/libs/objectcache/APCBagOStuff.php
index e41c3a2..1fedfaf 100644 (file)
--- a/includes/libs/objectcache/APCBagOStuff.php
+++ b/includes/libs/objectcache/APCBagOStuff.php
@@ -104,7 +104,7 @@ class APCBagOStuff extends BagOStuff {
                return $value;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                apc_delete( $key . self::KEY_SUFFIX );
 
                return true;

diff --git a/includes/libs/objectcache/APCUBagOStuff.php b/includes/libs/objectcache/APCUBagOStuff.php
index a26e560..fb43d4d 100644 (file)
--- a/includes/libs/objectcache/APCUBagOStuff.php
+++ b/includes/libs/objectcache/APCUBagOStuff.php
@@ -55,7 +55,7 @@ class APCUBagOStuff extends APCBagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                apcu_delete( $key . self::KEY_SUFFIX );
 
                return true;

diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php
index a7ef3d5..9635543 100644 (file)
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -262,8 +262,9 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         *
         * @param string $key
         * @return bool True if the item was deleted or not found, false on failure
+        * @param int $flags Bitfield of BagOStuff::WRITE_* constants
         */
-       abstract public function delete( $key );
+       abstract public function delete( $key, $flags = 0 );
 
        /**
         * Merge changes into the existing cache value (possibly creating a new one)

diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php
index 726836e..3927ed3 100644 (file)
--- a/includes/libs/objectcache/CachedBagOStuff.php
+++ b/includes/libs/objectcache/CachedBagOStuff.php
@@ -68,7 +68,7 @@ class CachedBagOStuff extends HashBagOStuff {
        }
 
        public function delete( $key, $flags = 0 ) {
-               parent::delete( $key );
+               parent::delete( $key, $flags );
                if ( !( $flags & self::WRITE_CACHE_ONLY ) ) {
                        $this->backend->delete( $key );
                }

diff --git a/includes/libs/objectcache/EmptyBagOStuff.php b/includes/libs/objectcache/EmptyBagOStuff.php
index 3f66c06..3bf58df 100644 (file)
--- a/includes/libs/objectcache/EmptyBagOStuff.php
+++ b/includes/libs/objectcache/EmptyBagOStuff.php
@@ -39,7 +39,7 @@ class EmptyBagOStuff extends BagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return true;
        }
 

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index ec66492..7d074fa 100644 (file)
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -106,7 +106,7 @@ class HashBagOStuff extends BagOStuff {
                return true;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                unset( $this->bag[$key] );
 
                return true;

diff --git a/includes/libs/objectcache/MemcachedBagOStuff.php b/includes/libs/objectcache/MemcachedBagOStuff.php
index f7bf86b..bf0da11 100644 (file)
--- a/includes/libs/objectcache/MemcachedBagOStuff.php
+++ b/includes/libs/objectcache/MemcachedBagOStuff.php
@@ -70,7 +70,7 @@ class MemcachedBagOStuff extends BagOStuff {
                        $value, $this->fixExpiry( $exptime ) );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return $this->client->delete( $this->validateKeyEncoding( $key ) );
        }
 

diff --git a/includes/libs/objectcache/MemcachedPeclBagOStuff.php b/includes/libs/objectcache/MemcachedPeclBagOStuff.php
index fe31c25..a6646bc 100644 (file)
--- a/includes/libs/objectcache/MemcachedPeclBagOStuff.php
+++ b/includes/libs/objectcache/MemcachedPeclBagOStuff.php
@@ -172,7 +172,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                return $this->checkResult( $key, parent::cas( $casToken, $key, $value, $exptime ) );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                $this->debugLog( "delete($key)" );
                $result = parent::delete( $key );
                if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND ) {

diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php
index 043f8cb..f549876 100644 (file)
--- a/includes/libs/objectcache/MultiWriteBagOStuff.php
+++ b/includes/libs/objectcache/MultiWriteBagOStuff.php
@@ -139,7 +139,7 @@ class MultiWriteBagOStuff extends BagOStuff {
                return $this->doWrite( $this->cacheIndexes, $asyncWrites, 'set', $key, $value, $exptime );
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'delete', $key );
        }
 

diff --git a/includes/libs/objectcache/RESTBagOStuff.php b/includes/libs/objectcache/RESTBagOStuff.php
index fc3d575..b0b82d8 100644 (file)
--- a/includes/libs/objectcache/RESTBagOStuff.php
+++ b/includes/libs/objectcache/RESTBagOStuff.php
@@ -124,16 +124,8 @@ class RESTBagOStuff extends BagOStuff {
                return false;
        }
 
-       /**
-        * Set an item
-        *
-        * @param string $key
-        * @param mixed $value
-        * @param int $exptime Either an interval in seconds or a unix timestamp for expiry
-        * @param int $flags Bitfield of BagOStuff::WRITE_* constants
-        * @return bool Success
-        */
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
+               // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [
                        'method' => 'PUT',
                        'url' => $this->url . rawurlencode( $key ),
@@ -146,13 +138,8 @@ class RESTBagOStuff extends BagOStuff {
                return $this->handleError( "Failed to store $key", $rcode, $rerr );
        }
 
-       /**
-        * Delete an item.
-        *
-        * @param string $key
-        * @return bool True if the item was deleted or not found, false on failure
-        */
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
+               // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [
                        'method' => 'DELETE',
                        'url' => $this->url . rawurlencode( $key ),

diff --git a/includes/libs/objectcache/RedisBagOStuff.php b/includes/libs/objectcache/RedisBagOStuff.php
index abf9e8b..a0febfc 100644 (file)
--- a/includes/libs/objectcache/RedisBagOStuff.php
+++ b/includes/libs/objectcache/RedisBagOStuff.php
@@ -132,7 +132,7 @@ class RedisBagOStuff extends BagOStuff {
                return $result;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                list( $server, $conn ) = $this->getConnection( $key );
                if ( !$conn ) {
                        return false;

diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php
index f2c3732..e2b9a52 100644 (file)
--- a/includes/libs/objectcache/ReplicatedBagOStuff.php
+++ b/includes/libs/objectcache/ReplicatedBagOStuff.php
@@ -90,8 +90,8 @@ class ReplicatedBagOStuff extends BagOStuff {
                return $this->writeStore->set( $key, $value, $exptime, $flags );
        }
 
-       public function delete( $key ) {
-               return $this->writeStore->delete( $key );
+       public function delete( $key, $flags = 0 ) {
+               return $this->writeStore->delete( $key, $flags );
        }
 
        public function add( $key, $value, $exptime = 0 ) {

diff --git a/includes/libs/objectcache/WinCacheBagOStuff.php b/includes/libs/objectcache/WinCacheBagOStuff.php
index 764230b..6b0bec0 100644 (file)
--- a/includes/libs/objectcache/WinCacheBagOStuff.php
+++ b/includes/libs/objectcache/WinCacheBagOStuff.php
@@ -45,7 +45,7 @@ class WinCacheBagOStuff extends BagOStuff {
                return ( is_array( $result ) && $result === [] ) || $result;
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
                wincache_ucache_delete( $key );
 
                return true;

diff --git a/includes/objectcache/SqlBagOStuff.php b/includes/objectcache/SqlBagOStuff.php
index 2866a6c..522233b 100644 (file)
--- a/includes/objectcache/SqlBagOStuff.php
+++ b/includes/objectcache/SqlBagOStuff.php
@@ -423,7 +423,9 @@ class SqlBagOStuff extends BagOStuff {
                return (bool)$db->affectedRows();
        }
 
-       public function delete( $key ) {
+       public function delete( $key, $flags = 0 ) {
+               $ok = true;
+
                list( $serverIndex, $tableName ) = $this->getTableByKey( $key );
                $db = null;
                $silenceScope = $this->silenceTransactionProfiler();
@@ -435,10 +437,13 @@ class SqlBagOStuff extends BagOStuff {
                                __METHOD__ );
                } catch ( DBError $e ) {
                        $this->handleWriteError( $e, $db, $serverIndex );
-                       return false;
+                       $ok = false;
+               }
+               if ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) {
+                       $ok = $this->waitForReplication() && $ok;
                }
 
-               return true;
+               return $ok;
        }
 
        public function incr( $key, $step = 1 ) {
@@ -711,14 +716,8 @@ class SqlBagOStuff extends BagOStuff {
                if ( $exception instanceof DBConnectionError ) {
                        $this->markServerDown( $exception, $serverIndex );
                }
-               $this->logger->error( "DBError: {$exception->getMessage()}" );
-               if ( $exception instanceof DBConnectionError ) {
-                       $this->setLastError( BagOStuff::ERR_UNREACHABLE );
-                       $this->logger->debug( __METHOD__ . ": ignoring connection error" );
-               } else {
-                       $this->setLastError( BagOStuff::ERR_UNEXPECTED );
-                       $this->logger->debug( __METHOD__ . ": ignoring query error" );
-               }
+
+               $this->setAndLogDBError( $exception );
        }
 
        /**
@@ -741,6 +740,13 @@ class SqlBagOStuff extends BagOStuff {
                        }
                }
 
+               $this->setAndLogDBError( $exception );
+       }
+
+       /**
+        * @param DBError $exception
+        */
+       private function setAndLogDBError( DBError $exception ) {
                $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
@@ -813,20 +819,26 @@ class SqlBagOStuff extends BagOStuff {
                }
 
                // Main LB is used; wait for any replica DBs to catch up
-               $masterPos = $lb->getMasterPos();
-               if ( !$masterPos ) {
-                       return true; // not applicable
-               }
+               try {
+                       $masterPos = $lb->getMasterPos();
+                       if ( !$masterPos ) {
+                               return true; // not applicable
+                       }
 
-               $loop = new WaitConditionLoop(
-                       function () use ( $lb, $masterPos ) {
-                               return $lb->waitForAll( $masterPos, 1 );
-                       },
-                       $this->syncTimeout,
-                       $this->busyCallbacks
-               );
+                       $loop = new WaitConditionLoop(
+                               function () use ( $lb, $masterPos ) {
+                                       return $lb->waitForAll( $masterPos, 1 );
+                               },
+                               $this->syncTimeout,
+                               $this->busyCallbacks
+                       );
 
-               return ( $loop->invoke() === $loop::CONDITION_REACHED );
+                       return ( $loop->invoke() === $loop::CONDITION_REACHED );
+               } catch ( DBError $e ) {
+                       $this->setAndLogDBError( $e );
+
+                       return false;
+               }
        }
 
        /**